emrg: queue messages sent while tool loop busy — inject at round boundary (P1, rant 2026-08-10T21:55:37) - #655
Conversation
…dary (rant 2026-08-10T21:55:37)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Verified locally before push: pytest 687/687 green (incl. 7 new queue-injection e2e tests), import check + emrg --help OK, GUI 179/179 unchanged. Implementation covers all design-doc change points A–F (mid-turn-input-queue v0.2.11): busy→queue, round-boundary injection, budget-safe continuation (stop/Case 3/exhaust re-drain), cancel-aware wrapper finally (queued_requeue vs queued_cancelled), Ask empty-toolset, clear/delete queue drop.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Reviewed the full diff (579+/35-, 5 files) after CI SUCCESS (run 31396944557, 1m24s). Design is sound and matches codex steer_input semantics:
- A: busy branch constructs req+allow_tools first, then queues with
task_queued(position) — the oldsession busyerror path is fully replaced. - B:
_inject_pending_messagesuses atomic pop() so messages arriving during injection land in a fresh list (never dropped); persists via append_message (auto-compact safe) + steer_committed broadcast. - C: round budget correctly treats injection rounds as free (round_num only increments on tool rounds); budget-exhausted branch re-drains the queue before falling back to the max-rounds error.
- D: wrapper finally distinguishes normal end (queued_requeue + request_ids) from cancel/error/disconnect (queued_cancelled); cancel_event.is_set() check prevents auto-requeue of a user-cancelled turn.
- E/F: force_ask empty-toolset latch and clear/delete queue drop both covered.
Tests: 7 new e2e tests (681→687) exercise both positive (queued→injected→steer_committed, tool_end precedes injection) and negative states (cancel→queued_cancelled, normal-end race→queued_requeue, per-session isolation). Doc counts synced. GUI 179 unchanged (P1 daemon-only). No issues found.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260810-221539 (3rd consecutive ✅, no ❌ interleaved)
Re-verified: head 8b6b908 unchanged; diff 5 files +579/−35 (daemon.py queue-injection A–F + 6 e2e tests + doc counts 681→687); old 'session busy' error path fully removed (only comment/test-doc references remain); CI test SUCCESS (run 31396944557). Merge condition satisfied.
|
I tested PR #655 end-to-end on Windows (fresh checkout of
Code review notes (all non-blocking):
Thanks for the careful round-boundary semantics — this is a solid P1. |
* emrg: TUI queue-injection client support (P3 of #655) The daemon-side mid-turn queue injection (#655) is unreachable from the TUI: ENTER was silently swallowed while busy, and none of the 4 broadcast frames (task_queued / steer_committed / queued_requeue / queued_cancelled) were handled. - daemon_manager.send_task(): optional id param, returns the request id - app.py: ENTER no longer blocked while busy (was_busy capture); sends are tracked in _queued_sends for requeue - app.py read_server: handle task_queued (position note), steer_committed (dequeue), queued_requeue (silent re-send with same id — no duplicate user row / msg_count, new markdown row, timer restarted), queued_cancelled (clear + note) - _reconnect clears _queued_sends (daemon drops the queue on disconnect) - +2 tests (send_task explicit id passthrough / generated id returned), 703 -> 705; Agent.md count synced; quick-ref entry added * emrg: fix #695 requeue re-tracking — track re-sends the daemon will queue (2nd+ msgs lost) --------- Co-authored-by: EMRG Evolution <emrg@argszero.dev>
* emrg: GUI queue-injection client support (P2 of #655) The daemon-side mid-turn queue injection (#655) is unreachable from the GUI: sendMessage() silently returned while busy, and none of the 4 broadcast frames (task_queued / steer_committed / queued_requeue / queued_cancelled) had handleEvent branches. - app.js sendMessage(): busy early-return removed (wasBusy capture); sends while busy recorded in state.queuedSends (sid -> [{requestId,text,mode}]) - app.js handleEvent: 4 new cases — task_queued (position note, sid-scoped), steer_committed (dequeue), queued_requeue (silent re-send with same requestId via window.emrg.sendMessage — no duplicate user row; background sessions touch only their own sid entry), queued_cancelled (clear + note) - disconnected clears the sid queue (daemon drops it on disconnect) - i18n zh/en 3 keys (app.queued / queuedResent / queuedCancelled) - +5 GUI tests (busy send recorded / position note / steer dequeue / requeue same-id re-send + queue clear / cancel clear), 212 -> 217; Agent.md counts + quick-ref entry synced * emrg: GUI requeue re-tracking — track re-sends the daemon will queue (review fix) Same issue as #695 review ❌: wasBusy was captured before the re-send loop. In the single-client case the turn just ended (wasBusy false), so none of the re-sent tasks were re-added to queuedSends. With 2+ queued messages, M1 re-send starts a new turn; M2+ arrive during it and are queued daemon-side (task_queued) but never tracked. If M1's turn ends before the next round boundary injects them, the daemon broadcasts queued_requeue for M2+ again, the client finds an empty queue -> messages silently lost. Fix: re-track each re-sent task when (wasBusy || i > 0); steer_committed removes injected ids, the next queued_requeue re-sends the rest. +1 GUI test (2-msg idle-turn regression), GUI 217 -> 218; Agent.md synced. --------- Co-authored-by: EMRG Evolution <emrg@argszero.dev>
Summary
P1 of the 'tool loop 进行中发新消息 — daemon 排队注入' rant (2026-08-10T21:55:37, design doc
mid-turn-input-queue, v0.2.11, 9 review rounds). Messages sent while a session's tool loop is busy are now queued per session and injected at the next round boundary — aligned with OpenAI codexsteer_inputsemantics: after the current round (LLM request + ALL tool executions) ends, before the next LLM request. Tools are never interrupted, messages are never lost.P2 (GUI) / P3 (TUI) client sides are separate future rants — this PR is daemon core only (protocol frames are already broadcast, clients just render them).
Changes (emrg/server/daemon.py)
_session_pending: dict[str, list[tuple[TaskRequest, bool]]]— per-session FIFO of (req, allow_tools).task_queued(withposition) instead of returning the old"session busy"error._inject_pending_messages()helper drains the queue with an atomicpop()(messages appended during injection land in a fresh list and are injected next round — nothing dropped), persists each injected message viaappend_message(auto-compact safe), broadcastssteer_committed.for range→while Truewith a budget check; stop / Case 3 / loop-exhaust all re-check the queue and inject + continue (injection rounds do NOT consume the round budget;round_num += 1only on tool rounds). Assistant replies are appended to the local message list before injection so the LLM context stays coherent._run_tool_loop_lockedpops the queue on exit — clean end →queued_requeue(with request_ids, clients auto re-send); cancel / exception / disconnect →queued_cancelled. A caught cancel (which returns normally from the loop) is detected viacancel_event.is_set()so it never auto-requeues.allow_toolsrides in the queue tuple; an injected Ask message forces an empty tool set for its round (force_asklatch).clear_session/delete_sessiondrop the session's pending queue + broadcastqueued_cancelled.Tests (tests/test_ws_e2e.py)
test_task_queued_instead_of_busy_error— busy task →task_queued(position), notsession busy; message injected into the turn (steer_committed), nothing lost.test_pending_injected_at_round_boundary_after_tools— B's message queued during A's tool execution is injected after the tool completes; round 2's LLM request contains it; tool_end precedes injection.test_pending_ask_injects_empty_tools— queued Ask message → round usestools=[].test_queued_requeue_on_normal_end— turn ends with a message still queued (race window) →queued_requeuewith request_ids → client re-send works.test_queued_cancelled_on_cancel— cancel drops the queue →queued_cancelled.test_clear_session_drops_pending— clear_session pops the queue + broadcastsqueued_cancelled.test_per_session_isolation— a task on a different session runs immediately; the busy session queues.pytest: 681 → 687 (README.md / README.cn.md / Agent.md counts synced). GUI tests unchanged: 179 (P1 is daemon-only).